Skip to content

feat: interactive commands support #22

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
nxtcoder17 merged 1 commit into from
Nov 10, 2024
Merged

feat: interactive commands support #22

nxtcoder17 merged 1 commit into from
Nov 10, 2024

Conversation

@nxtcoder17
Copy link
Owner

@nxtcoder17 nxtcoder17 commented Nov 10, 2024 •
edited
Loading

fixes issue with dumping stdout and stderr of commands, there was a bug in current implementation, as goroutines for reading and dumping stdout ere not going through a waitgroup, which caused them to abruptly exit, without logging the remaining stuffs

Resolves #21

image

Summary by Sourcery

Introduce support for interactive commands by adding an 'interactive' flag to task configurations. Fix a bug related to premature goroutine exits when handling stdout and stderr. Enhance logging by adding task-specific prefixes and improve handling of line breaks. Add tests to validate the new interactive task functionality.

New Features:

  • Add support for interactive commands by introducing an 'interactive' flag in the task configuration.

Bug Fixes:

  • Fix issue with goroutines for reading and dumping stdout and stderr not using a waitgroup, which caused premature exits without logging remaining output.

Enhancements:

  • Improve logging of stdout and stderr by adding task-specific prefixes and handling line breaks more effectively.

Tests:

  • Add test cases to verify the functionality of interactive tasks, ensuring that tasks with the 'interactive' flag behave as expected.

@sourcery-ai
Copy link

sourcery-ai bot commented Nov 10, 2024 •
edited
Loading

Reviewer's Guide by Sourcery

This PR implements interactive command support and fixes a bug in stdout/stderr handling. The main changes involve adding a waitgroup to properly manage goroutines for stdout/stderr processing and introducing a new 'interactive' flag for tasks that require user input.

Sequence diagram for stdout/stderr handling with waitgroup

sequenceDiagram
    participant Task
    participant StdoutGoroutine
    participant StderrGoroutine
    participant WaitGroup

    Task->>WaitGroup: Add goroutines
    Task->>StdoutGoroutine: Start stdout goroutine
    Task->>StderrGoroutine: Start stderr goroutine

    StdoutGoroutine->>StdoutGoroutine: Read stdout
    StdoutGoroutine->>Task: Write to stdout
    StdoutGoroutine->>WaitGroup: Done

    StderrGoroutine->>StderrGoroutine: Read stderr
    StderrGoroutine->>Task: Write to stderr
    StderrGoroutine->>WaitGroup: Done

    WaitGroup->>Task: Wait for goroutines to finish
Loading

Updated class diagram for cmdArgs and ParsedTask

classDiagram
    class cmdArgs {
        +string cmd
        +bool interactive
        +io.Writer stdout
        +io.Writer stderr
    }

    class ParsedTask {
        +string[] Shell
        +string WorkingDir
        +map<string, string> Env
        +bool Interactive
        +CommandJson[] Commands
    }

    note for cmdArgs "Added 'interactive' flag to support interactive commands"
    note for ParsedTask "Added 'Interactive' attribute to handle interactive tasks"
Loading

File-Level Changes

Change Details Files
Improved stdout/stderr handling with proper goroutine synchronization
  • Added WaitGroup to synchronize stdout/stderr reading goroutines
  • Replaced ReadBytes with single byte reading for more granular output control
  • Added proper closing of stdout/stderr writers
  • Improved prefix handling for output lines
  • Fixed EOF handling in output streams
 pkg/runfile/run.go
Added interactive mode support for tasks
  • Added 'interactive' field to Task and ParsedTask structs
  • Added stdin handling for interactive commands
  • Updated command creation to support interactive mode
 pkg/runfile/task.go
pkg/runfile/task-parser.go
pkg/runfile/run.go
Added test coverage for interactive tasks
  • Added test case for interactive task parsing
  • Updated task comparison logic to check interactive flag
 pkg/runfile/task-parser_test.go

Possibly linked issues

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time. You can also use
    this command to specify where the summary should be inserted.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

sourcery-ai[bot]
sourcery-ai bot reviewed Nov 10, 2024
View reviewed changes
Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @nxtcoder17 - I've reviewed your changes - here's some feedback:

Overall Comments:

  • Consider removing the commented out debug logging lines to keep the code clean
Here's what I looked at during the review
  • 🟡 General issues: 1 issue found
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟡 Complexity: 1 issue found
  • 🟢 Documentation: all looks good
Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@nxtcoder17
feat: interactive commands support
652ecf4 
fixes issue with dumping stdout and stderr of commands, there was a bug
in current implementation, as goroutines for reading and dumping stdout
ere not going through a waitgroup, which caused them to abruptly exit,
without logging the remaining stuffs
@nxtcoder17 nxtcoder17 force-pushed the feat/interactive-commands branch from a6a8b79 to 652ecf4 Compare November 10, 2024 16:32
Repository owner deleted a comment from sourcery-ai bot Nov 10, 2024
Repository owner deleted a comment from sourcery-ai bot Nov 10, 2024
@nxtcoder17 nxtcoder17 merged commit 83fb82a into master Nov 10, 2024
@nxtcoder17 nxtcoder17 deleted the feat/interactive-commands branch November 10, 2024 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sourcery-ai sourcery-ai[bot] sourcery-ai[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[FEAT] support for interactive commands

2 participants

@nxtcoder17